Monumental Monsteras #15
Conversation
Create new readme
…be changed later.
… Currently requires a noqa since the action of importing causes the @ui.pages decorator to make the page, but that makes F401 (unused import) unhappy.
…nts to RPG style input
This time instead of using absolute positioning hacks it uses grid hacks to overlay the foreground over the background
Add input view component
This commit firstly makes the input_method_proto module, which defines an interface for input methods to follow. The interface is to allow the WPM-test page module to only have to worry about one implementation of input method rather than having special handling for each one.
Fix some pre-commit related errors in README.md
Add controller style input method
i forgot to import
…-in-rpg_text_input Switch comma for exclamation mark in rpg_text_input
…adable Make rpg_style_input keys more readable
Update keyboard docstring to remove raises
…ters 82 platformer support capital letters
Remove circle input method
Fix config.py platformer entry formatting
…-file-and-alter-the-file-imports-to-match Create color style class and refactor
Make platformer work with wasd and spacebar
Add logo icon and title
ZeroIntensity
left a comment
ZeroIntensity
left a comment
There was a problem hiding this comment.
This is a really well-done project. The game is fun to play, it's an interesting concept, and I can tell that a lot of thought and teamwork went into creating this. High-level, I don't really have anything bad to say about it.
In terms of code quality, here are the things that I primarily noticed:
- Dataclasses are really powerful, use them! There are several places where tuples or dictionaries were used where a dataclass would have been perfect. Attributes are almost always better than indexes.
- When appending to text, use
io.StringIOover+=. This is a mutable buffer that can be converted to a string later, while+=creates a copy of the string every time. - There are no test cases here. I understand that time was limited and test cases are difficult to write for web applications, but even something really simple using Selenium would have been great.
|
|
||
| COLOR_STYLE = ColorStyle() | ||
|
|
||
| media = Path("./static") |
There was a problem hiding this comment.
Like COLOR_STYLE above, this should be in SCREAMING_SNAKE_CASE as it's a module-level constant.
| COLOR_STYLE = ColorStyle() | ||
|
|
||
| media = Path("./static") | ||
| app.add_media_files("/media", media) |
There was a problem hiding this comment.
It's generally not a great idea to do IO upon import, but there's probably no great way around it here.
| media = Path("./static") | ||
| app.add_media_files("/media", media) | ||
|
|
||
| char_selection = [string.ascii_uppercase, string.ascii_lowercase, string.punctuation] |
| self.spin_task = None | ||
|
|
||
| self.intro_card, self.start_button = self.create_intro_card() | ||
| self.main_content, self.record, self.label, self.buttons_row, self.buttons_row_2 = self.create_main_content() |
There was a problem hiding this comment.
These assignments should be split across multiple lines.
| start_button = ui.button("Get started!", color=COLOR_STYLE.primary) | ||
| return intro_card, start_button | ||
|
|
||
| def create_main_content(self) -> tuple[ui.column, ui.image, ui.chip, ui.row, ui.row]: |
There was a problem hiding this comment.
After about two elements, it would be better to use a NamedTuple or a dataclass.
| return sentence[:-1] + secrets.choice(punctuation) | ||
|
|
||
|
|
||
| async def wpm_tester_page(method: str) -> None: |
There was a problem hiding this comment.
This doesn't have any await calls, so it doesn't need to be async.
| # Sidebar | ||
| with ( | ||
| ui.right_drawer(value=False, fixed=False) | ||
| .style(f"background-color: {COLOR_STYLE.secondary_bg}") | ||
| .props("overlay") | ||
| .classes("p-0") as right_drawer, | ||
| ui.element("q-scroll-area").classes("fit"), | ||
| ): | ||
| # Home nav button | ||
| with ( | ||
| ui.list().classes("fit"), | ||
| ui.item(on_click=lambda: ui.navigate.to("/")) | ||
| .props("clickable") | ||
| .classes(f"hover:bg-[{COLOR_STYLE.primary}]"), | ||
| ui.item_section(), | ||
| ): | ||
| ui.label("HOME").style(f"color: {COLOR_STYLE.contrast}") | ||
|
|
||
| with ui.list().classes("fit"), ui.column().classes("w-full items-center"): | ||
| ui.separator().style("background-color: #313131; width: 95%;") | ||
|
|
||
| # Input method nav buttons | ||
| with ui.list().classes("fit"): | ||
| for input_method in INPUT_METHODS: | ||
| path = f"/test/{input_method['path']}" | ||
| with ( | ||
| ui.item(on_click=lambda _, p=path: ui.navigate.to(p)) | ||
| .props("clickable") | ||
| .classes(f"hover:bg-[{COLOR_STYLE.primary}]"), | ||
| ui.item_section(), | ||
| ): | ||
| ui.label(input_method["name"].upper()).style(f"color: {COLOR_STYLE.contrast}") |
There was a problem hiding this comment.
Instead of simple comments, this would have benefited from being split up into smaller functions. For example, there could be one for the header, and then another for the sidebar, and so on.
| """flex flex-col justify-evenly items-center absolute w-[90vw] h-[85vh] left-1/2 top-1/2 | ||
| transform -translate-x-1/2 -translate-y-1/2 rounded-xl""" |
There was a problem hiding this comment.
To split across multiple lines, you don't need a triple-quoted string. If you have strings on multiple lines with nothing in between, Python will concatenate them for you. For example:
string = (
"abc"
"def"
)
assert string == "abcdef"| """Apply gravity and vertical player clamping.""" | ||
| self._yvel += constants.GRAVITY_FORCE * self._deltatime | ||
| dy = self._yvel * self._deltatime | ||
| if dy != 0: |
There was a problem hiding this comment.
Same comments from above about dy and a guard clause.
| """Gravity force""" | ||
| GRAVITY_FORCE = 17 | ||
|
|
||
| COLOR_PLAYER = "purple" |
No description provided.